Skip to content

Conversation

@esrakartalOpt
Copy link
Contributor

@esrakartalOpt esrakartalOpt commented Nov 5, 2025

Summary

  • Added holdout support for decision service

Test plan

PR checks

Issues

@Mat001
Copy link
Contributor

Mat001 commented Nov 7, 2025

@esrakartalOpt
I get a new issue in this PR, this time in optimizely.py. Check this:

Problem: For holdout decisions, variation is a dict, not an entities.Variation object.
The code accesses .key attribute which will raise AttributeError.

LINE 1234-1238 - CURRENT CODE (WRONG for holdouts)

variation_key = (
flag_decision.variation.key # ❌ Assumes variation is an object
if flag_decision is not None and flag_decision.variation is not None
else None
)

Why This Fails:

  • For regular experiments/rollouts: variation is entities.Variation with .key
    attribute ✅
  • For holdouts: variation is a dict with ['key'] accessor ❌
  • Accessing .key on a dict raises AttributeError

Proof:

  • Bucketer returns dict for holdouts (bucketer.py:147): variation = next((v for v in
    variations if v.get('id') == variation_id), None)
  • This dict variation is stored in Decision (decision_service.py:830):
    variation=variation

Similar Code that properly handles this (lines 1250-1253):
try:
if flag_decision.variation is not None:
variation_id = flag_decision.variation.id # Will fail for dict
except AttributeError:
self.logger.warning("flag_decision.variation has no attribute 'id'")

Required Fix:

Option 1: Handle both dict and object

try:
if flag_decision.variation is not None:
variation_key = (
flag_decision.variation['key']
if isinstance(flag_decision.variation, dict)
else flag_decision.variation.key
)
else:
variation_key = None
except (AttributeError, KeyError, TypeError):
self.logger.warning("Unable to extract variation key")
variation_key = None

Option 2: Add check for holdout source

if flag_decision.source == enums.DecisionSources.HOLDOUT:
variation_key = flag_decision.variation['key'] if flag_decision.variation else
None
else:
variation_key = flag_decision.variation.key if flag_decision.variation else None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants